-
Notifications
You must be signed in to change notification settings - Fork 736
Generic PackageArgument #6856
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generic PackageArgument #6856
Conversation
src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/PackageArgumentParserUtility.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/PackageArgumentParserUtility.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/PackageArgumentParserUtility.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/PackageArgumentParserUtility.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/PackageArgumentParserUtility.cs
Outdated
Show resolved
Hide resolved
|
|
||
| namespace NuGet.CommandLine.XPlat.Utility | ||
| { | ||
| internal static class PackageArgumentFactoryUtility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The factory isn't needed for generics, instead you can create a static method on the class and have it create typed instances
https://dotnetfiddle.net/jFHQhx
public static PackageArgument<TVersion> Create(string id, TVersion version, IEqualityComparer<TVersion?> versionComparer)
{
return new PackageArgument<TVersion>(versionComparer)
{
Id = id,
Version = version,
};
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. that’s a really good point. Having a static method on the generic class would definitely simplify the design and make it more flexible.
The reason I introduced the factory, though, was to make it easier for callers so they don’t have to remember to pass in the comparer every time. It ensures consistency by automatically using the right comparer for NuGetVersion and VersionRange cases.
|
Thank you for the reviews. I will be closing this PR since we went ahead with the other implementation. |
Bug
Fixes:
Description
This PR introduces a generic
PackageArgumenttype. Previously, we hadPackagetype which only supported VersionRange versions. This new generic enables callers to implement it with both NuGetVersion and VersionRange.PR Checklist